scripts/ai-review.ts (3)

55-55: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add timeouts to both fetch calls.

Neither the Gemini API call (line 55) nor the GitHub API call (line 92) has a timeout. If either endpoint hangs, the workflow blocks until the 5-minute job timeout. Adding an AbortController with a reasonable timeout (e.g., 30s for Gemini, 10s for GitHub) fails fast and improves reliability.

⏱️ Proposed fix for both fetch calls

+    const geminiController = new AbortController();
+    const geminiTimeout = setTimeout(() => geminiController.abort(), 30000);
     const response = await fetch(`https://generativelanguage.googleapis.com/...`, {
       method: 'POST',
       headers: { ... },
       body: JSON.stringify({ ... })
+      , signal: geminiController.signal
     });
+    clearTimeout(geminiTimeout);

+    const ghController = new AbortController();
+    const ghTimeout = setTimeout(() => ghController.abort(), 10000);
     const ghResponse = await fetch(`https://api.github.com/...`, {
       method: 'POST',
       headers: { ... },
       body: JSON.stringify({ ... })
+      , signal: ghController.signal
     });
+    clearTimeout(ghTimeout);

Also applies to: 92-92

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ai-review.ts` at line 55, Add AbortController-based timeouts to both
fetch calls in the script: use a roughly 30-second timeout for the Gemini
request and 10 seconds for the GitHub request, pass each controller’s signal to
fetch, and ensure timers are cleared after completion while preserving existing
error handling.

---

24-24: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Use spawn with an argument array instead of string interpolation in execSync.

BASE_SHA and HEAD_SHA come from GitHub context (valid hex SHAs), so the actual injection risk is low. However, string interpolation in execSync is a command-injection anti-pattern. Using spawnSync with an argument array eliminates the risk entirely and follows defense-in-depth principles.

🛡️ Proposed fix

-import { execSync } from 'child_process';
+import { spawnSync } from 'child_process';

 // ...

-    const diff = execSync(`git diff ${BASE_SHA} ${HEAD_SHA}`).toString();
+    const result = spawnSync('git', ['diff', BASE_SHA!, HEAD_SHA!], { encoding: 'utf-8' });
+    if (result.status !== 0) {
+      console.error(`git diff failed: ${result.stderr}`);
+      process.exit(1);
+    }
+    const diff = result.stdout;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ai-review.ts` at line 24, Replace the interpolated execSync call in
the diff retrieval logic with spawnSync (or spawn) using an argument array
containing "git", "diff", BASE_SHA, and HEAD_SHA; preserve synchronous output
handling and convert the returned stdout to a string as before.

Source: Linters/SAST tools

---

3-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Validate all required environment variables, not just credentials.

PR_NUMBER, REPO, BASE_SHA, and HEAD_SHA are used without validation. If any are missing, the script produces confusing errors (e.g., git diff undefined undefined or a malformed GitHub API URL). Adding early checks alongside the existing credential checks improves debuggability.

♻️ Proposed fix

 if (!GEMINI_API_KEY) {
   console.log("GEMINI_API_KEY is not set. Skipping AI review.");
   process.exit(0);
 }

 if (!GITHUB_TOKEN) {
   console.log("GITHUB_TOKEN is not set. Skipping AI review.");
   process.exit(0);
 }

+const requiredVars = { PR_NUMBER, REPO, BASE_SHA, HEAD_SHA };
+for (const [name, value] of Object.entries(requiredVars)) {
+  if (!value) {
+    console.error(`${name} is not set. Aborting.`);
+    process.exit(1);
+  }
+}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ai-review.ts` around lines 3 - 8, Validate PR_NUMBER, REPO, BASE_SHA,
and HEAD_SHA at startup alongside GEMINI_API_KEY and GITHUB_TOKEN in
scripts/ai-review.ts; fail early with a clear error identifying each missing
required environment variable before any Git or GitHub operations run.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

scripts/ai-review.ts (1)

31-31: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Optional: prefer execFileSync with argument array over string interpolation.

BASE_SHA/HEAD_SHA come from github.event.pull_request.*.sha (trusted 40-char hashes), so injection is unlikely, but passing them as arguments avoids the shell entirely and silences the child_process warning.

♻️ Proposed change

-    const diff = execSync(`git diff ${BASE_SHA} ${HEAD_SHA}`).toString();
+    const diff = execFileSync('git', ['diff', BASE_SHA, HEAD_SHA]).toString();

(update the import to execFileSync accordingly)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ai-review.ts` at line 31, Replace the interpolated shell command in
the diff-loading logic with execFileSync and an argument array containing the
git executable arguments and BASE_SHA/HEAD_SHA; update the child_process import
accordingly while preserving the existing string conversion and diff behavior.

Source: Linters/SAST tools

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

scripts/ai-review.ts (1)

65-71: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Mistral response extraction still uses incompatible response shape.

This was flagged in a prior review and remains unresolved. client.beta.conversations.start() returns a ConversationResponse with an outputs array, not choices or top-level message.content. The current extraction will always yield an empty string, causing the workflow to exit with status 1.

🐛 Proposed fix: extract from `response.outputs`

-      reviewComment = (response as any)?.choices?.[0]?.message?.content || (response as any)?.message?.content || "";
+      const outputs = (response as any)?.outputs;
+      const messageOutput = outputs?.find((o: any) => o.type === 'message');
+      reviewComment = messageOutput?.content?.map((c: any) => c.text).join('') || "";

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ai-review.ts` around lines 65 - 71, Update the response extraction
after client.beta.conversations.start() to read the generated review text from
the ConversationResponse outputs array instead of choices or top-level
message.content. Use the appropriate content field from the first output,
preserving a safe empty-string fallback, so reviewComment receives the model
response.

scripts/ai-review.ts (1)

30-30: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Prefer execFileSync with argument array over execSync with string interpolation.

execSync runs through a shell, so interpolated BASE_SHA/HEAD_SHA are command-injection vectors if those env vars are ever sourced from untrusted input. Use execFileSync to bypass the shell entirely.

🔒️ Proposed refactor

-    const diff = execSync(`git diff ${BASE_SHA} ${HEAD_SHA}`).toString();
+    const diff = execFileSync('git', ['diff', BASE_SHA, HEAD_SHA]).toString();

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/ai-review.ts` at line 30, Replace the interpolated execSync call in
the diff retrieval logic with execFileSync, passing the Git arguments as a
separate array containing BASE_SHA and HEAD_SHA; update the child_process import
accordingly and preserve the existing string conversion.

Source: Linters/SAST tools